feat(terraform): add the Azure onyx composition and README by justin-tahara · Pull Request #14105 · onyx-dot-app/onyx

justin-tahara · 2026-08-19T22:19:33Z

Stacked PR 7 of 7. Each PR targets the branch below it, so GitHub retargets the next one to main as each merges. Review and merge bottom-up.

feat(terraform): add the Azure vnet module #14099 — vnet, network, subnets, NAT gateway

feat(terraform): add the Azure storage module #14100 — storage, storage account + container

feat(terraform): add the Azure postgres module #14101 — postgres, flexible server + alerts

feat(terraform): add the Azure redis module #14102 — redis, cache + private endpoint

feat(terraform): add the Azure aks module #14103 — aks, cluster + workload identity

feat(terraform): add the Azure waf module #14104 — waf, regional WAF policy

feat(terraform): add the Azure onyx composition and README #14105 — onyx, composition + README ← this PR

Description

Wires the six modules together with t-shirt sizing, and documents the set. Completes the Azure equivalent of deployment/terraform/modules/aws.

Sizing picks the Azure size closest to what the AWS composition chooses at each tier, with one deliberate departure: the index pool is memory-optimised at every tier, because on Azure it carries the document index itself. There is no managed OpenSearch to move that load off the cluster.

	small	medium	large
system pool VM	`Standard_D8ds_v5`	`Standard_D16ds_v5`	`Standard_D16ds_v5`
system pool nodes	1-3	1-5	2-8
index pool VM	`Standard_E4ds_v5`	`Standard_E8ds_v5`	`Standard_E16ds_v5`
index pool disk	256 GiB	512 GiB	1024 GiB
database SKU	`GP_Standard_D2ds_v5`	`GP_Standard_D2ds_v5`	`GP_Standard_D4ds_v5`
database storage	64 GiB	128 GiB	256 GiB
cache	Standard C3 (6 GB)	Standard C4 (13 GB)	Standard C5 (26 GB)

Two things the composition does that no single module can:

Flow logs get a storage account of their own. That keeps the dependency one-way — log account, then network, then the file store account that restricts itself to the network's subnets — where pointing the network at the file store account would have made a cycle.
Egress picks itself. With a created network and a NAT gateway the cluster routes through it and keeps one address; bringing a network without one falls back to letting AKS manage outbound.

Unlike the AWS composition this module declares no provider block. The azurerm provider needs a features block and a subscription, and both belong to the root module rather than to a module that might be counted.

One deliberate parity gap, for a decision

Flow logs are off by default, where the AWS modules have them on. Azure writes them to a storage account and needs a Network Watcher in the region, so a default of on would fail an apply on any subscription without one — and Azure only auto-creates the Watcher if the subscription has not opted out. I chose a working default over matching AWS, and the README documents the one variable that turns them on. Happy to flip it if you would rather fail loudly than ship less logging than AWS.

How Has This Been Tested?

cd deployment/terraform/modules/azure/onyx
terraform init -backend=false && terraform test
# Success! 14 passed, 0 failed.

All seven modules together: 85 passed, 0 failed.

The composition suite covers every sizing tier, per-variable overrides beating tier defaults without disturbing their neighbours, the derived storage account name staying inside Azure's 24-character lowercase-alphanumeric rule (including from a long prefix), the resource group create/join paths, and the flow-log account.

This suite caught a real defect in the aks module that its own tests could not: azurerm_role_assignment used for_each over storage account IDs that are unknown at plan time when they come from a storage module in the same apply, which fails terraform plan outright on any greenfield deployment. Fixed in PR 5 of this stack.

I also cross-checked the README against the code rather than trusting it: every module.onyx.* output it references exists, and all four AZURE_* environment variables it names are read by backend/onyx/configs/app_configs.py and exposed in deployment/helm/charts/onyx/values.yaml.

Not applied against a live subscription. The README says so up front. Mocked plans prove the wiring and the input validation; only an apply proves the modules work.

Additional Options

[Optional] Please cherry-pick this PR to the latest release version.
[Optional] Override Linear Check

Changes from review (greptile, cubic)

The Helm section was wrong in a way that would have failed at runtime. Following it gave workloads the default service account, so DefaultAzureCredential got no token and every file-store call failed to authenticate. Rewritten into three concrete steps: point serviceAccount.name at the federated account; add the azure.workload.identity/use pod label to the API server and every Celery worker (it is per-pod, and the webhook will not project a token without it); and give the OpenSearch subchart the nodeSelector and tolerations for the tainted index pool. I checked the chart — it sets none of these, so the index pool would otherwise have sat empty while OpenSearch scheduled onto the system pool.
A database password is now required unless entra_database_authentication_only is set, which the composition now supports end to end.
A public API server with no authorized ranges is now refused, matching the aks module.
aks_outbound_type lets a supplied subnet that already has a NAT gateway keep its stable egress address, instead of being forced onto loadBalancer.
Flow logs are rejected with a supplied network, where they previously created a log storage account nothing would ever write to.
README fix: the existing-network example referenced var.postgres_password while the quickstart defines a local.

Tests: 14 → 21. Across all seven modules: 120 passed, 0 failed (was 85).

Round 2

The Helm instructions now require the onyx namespace. A service account belongs to one namespace, and the federated one is created in onyx. A release installed anywhere else references an account that does not exist there, and the API and Celery pods never start.
The flow-log storage account name no longer loses its digest. It carries an extra log, so at a full-length prefix the 24-character truncation kept only three of the six digest characters — two deployments could collide in Azure's global namespace. Its prefix is now 15 characters, so 15 + 3 + 6 = 24 and the digest survives whole. The file store name was already fine at 18 + 6.
nat_gateway_public_ips says what it actually reflects — a NAT gateway this module created, not egress in general.
postgres_password description names the right variable. enable_entra_database_authentication only adds Entra logins alongside passwords; entra_database_authentication_only is the one that removes the need for a password.
Dropped a tautological assertion that restated the run's own input and could never fail.

Tests: 21 → 22. Across all seven modules: 137 passed, 0 failed (was 120).

Round 3

The workload namespace ordering bug is fixed in PR feat(terraform): add the Azure aks module #14103, where the service account is created. On a first deployment Terraform made the account in the onyx namespace before anything had made that namespace, and the apply failed. The Helm install could not have satisfied it either, since it runs afterwards.
The documented Helm command no longer passes --create-namespace. Terraform owns the namespace now, so the release joins it. create_workload_namespace is exposed here to opt out.

Tests: 22. Across all seven modules: 140 passed, 0 failed.

greptile-apps · 2026-08-19T22:22:15Z

Greptile Summary

The PR adds an Azure Terraform composition that wires networking, storage, PostgreSQL, Redis, AKS, and WAF modules together with tiered sizing and deployment guidance.

Adds small, medium, and large infrastructure presets with per-variable overrides.
Supports created or supplied networks, optional flow logs, NAT-aware AKS egress, password or Entra-only PostgreSQL authentication, and workload identity.
Documents Terraform provisioning and the subsequent namespace-bound Helm installation.

Confidence Score: 4/5

The PR is not yet safe to merge because disabling Terraform-owned service-account creation can also suppress the namespace required by the documented Helm installation.

The composition exposes independent namespace and service-account ownership flags, but the AKS module couples namespace creation to service-account creation; a supported chart-owned service-account configuration therefore leaves Helm targeting a namespace that was never created.

Files Needing Attention: deployment/terraform/modules/azure/onyx/variables.tf, deployment/terraform/modules/azure/onyx/main.tf

Important Files Changed

Filename	Overview
deployment/terraform/modules/azure/onyx/main.tf	Composes the six Azure modules and derives sizing, networking, identity, and storage settings; forwarding separate namespace and service-account ownership flags exposes an invalid ownership combination.
deployment/terraform/modules/azure/onyx/variables.tf	Defines and validates composition inputs, but create_workload_service_account=false can unexpectedly defeat the independently enabled namespace creation contract.
deployment/terraform/modules/azure/README.md	Documents provisioning and workload-identity-aware Helm installation, which relies on Terraform creating the onyx namespace.
deployment/terraform/modules/azure/onyx/tests/onyx.tftest.hcl	Covers sizing, naming, networking, authentication, and exposure validation but not independent namespace and service-account ownership combinations.
deployment/terraform/modules/azure/onyx/outputs.tf	Exposes cluster, storage, database, Redis, WAF, and credential values required by downstream deployment.
deployment/terraform/modules/azure/onyx/versions.tf	Declares the Terraform and provider requirements for the composition.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Root[Root Terraform module] --> Onyx[Azure Onyx composition]
  Onyx --> VNet[VNet and subnets]
  Onyx --> Storage[Blob file store]
  Onyx --> Postgres[PostgreSQL Flexible Server]
  Onyx --> Redis[Azure Cache for Redis]
  Onyx --> AKS[AKS cluster]
  Onyx --> WAF[WAF policy]
  VNet --> AKS
  VNet --> Postgres
  VNet --> Redis
  Storage --> AKS
  AKS --> Namespace[onyx namespace]
  Namespace --> ServiceAccount[Federated service account]
  ServiceAccount --> Helm[Onyx Helm workloads]

Prompt To Fix All With AI

### Issue 1
deployment/terraform/modules/azure/onyx/variables.tf:396-399
**Service-account opt-out drops namespace**

When a caller sets `create_workload_service_account=false` so Helm can create the service account while leaving `create_workload_namespace=true`, the AKS module also skips namespace creation. The documented Helm command does not pass `--create-namespace`, so installation fails because the `onyx` namespace does not exist.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (4): Last reviewed commit: "feat(terraform): add the Azure onyx comp..." | Re-trigger Greptile}

cubic-dev-ai

6 issues found across 6 files

Confidence score: 2/5

deployment/terraform/modules/azure/README.md does not configure file-store pods with the federated onyx-workload-access service account and workload-identity annotation, so DefaultAzureCredential can fail at runtime; update the chart pod templates before relying on federated access.
deployment/terraform/modules/azure/onyx/main.tf enables PostgreSQL password authentication when postgres_password is omitted, causing Azure apply failures; require a password or disable password authentication.
The default AKS settings in deployment/terraform/modules/azure/variables.tf expose the API server publicly without authorized IP ranges, creating a significant security exposure; default to a private or restricted API server configuration.
Existing-network configurations in deployment/terraform/modules/azure/onyx/main.tf can silently omit flow logs and override an existing NAT gateway’s stable egress behavior, while the README example also references an undefined var.postgres_password; validate these combinations and correct the example before documenting the path.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="deployment/terraform/modules/azure/onyx/main.tf">

<violation number="1" location="deployment/terraform/modules/azure/onyx/main.tf:146">
P2: When `create_virtual_network = false` and `enable_flow_logs = true`, this creates a dedicated account but no flow-log resource, so logging is silently absent. Reject this combination or configure flow logs for the supplied VNet instead of creating an unused account.</violation>

<violation number="2" location="deployment/terraform/modules/azure/onyx/main.tf:189">
P1: When callers omit `postgres_password`, the composition plans with password authentication enabled but Azure rejects the server at apply. Require a non-null password or disable password authentication before invoking `module.postgres`.</violation>

<violation number="3" location="deployment/terraform/modules/azure/onyx/main.tf:240">
P2: When a supplied AKS subnet already has a NAT gateway, `create_virtual_network = false` still forces AKS to use `loadBalancer`, so egress can change instead of using the existing stable address. Add an explicit existing-network NAT/outbound setting or fail when stable egress is requested.</violation>
</file>

<file name="deployment/terraform/modules/azure/onyx/variables.tf">

<violation number="1" location="deployment/terraform/modules/azure/onyx/variables.tf:278">
P1: The default composition deploys a public AKS API server open to any address: private_cluster_enabled defaults to false and api_server_authorized_ip_ranges to [], and the aks module only forbids combining them, it never requires one. The module's own README lists this as a security requirement. Add a validation on private_cluster_enabled (or api_server_authorized_ip_ranges) that fails the plan unless one is set, instead of shipping an open control plane until a user reads the docs.</violation>
</file>

<file name="deployment/terraform/modules/azure/README.md">

<violation number="1" location="deployment/terraform/modules/azure/README.md:147">
P3: In the 'Using an existing network' example, postgres_password = var.postgres_password references a variable that nothing in this README defines; the quickstart just above uses a local (postgres_password = local.postgres_password). A reader copy-pasting both gets an undefined-variable error. Reference the local instead to match the established example.</violation>

<violation number="2" location="deployment/terraform/modules/azure/README.md:252">
P1: Configure the chart's file-store workloads to use the federated `onyx-workload-access` service account and add `azure.workload.identity/use: "true"` to their pod templates before relying on `DefaultAzureCredential`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

cubic-dev-ai

5 issues found across 6 files

Confidence score: 2/5

deployment/terraform/modules/azure/README.md does not require the Helm release to use the onyx namespace, so the namespace-scoped onyx-workload-access ServiceAccount may not be available to the workload; install with --namespace onyx --create-namespace.
deployment/terraform/modules/azure/onyx/main.tf truncates long normalized prefixes enough to retain only three digest characters, allowing distinct deployments to collide in Azure’s global storage-account namespace; reserve space for the full six-character digest.
deployment/terraform/modules/azure/onyx/outputs.tf reports an empty nat_gateway_public_ips list when create_virtual_network=false, which can produce incomplete consumer allowlists despite the supplied subnet using a NAT gateway; expose the relevant NAT gateway IPs in this path.
deployment/terraform/modules/azure/onyx/variables.tf misstates how enable_entra_database_authentication affects password logins, while deployment/terraform/modules/azure/tests/onyx.tftest.hcl has a tautological password assertion; correct the description and make the test verify the module’s behavior rather than its input.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="deployment/terraform/modules/azure/onyx/outputs.tf">

<violation number="1" location="deployment/terraform/modules/azure/onyx/outputs.tf:87">
P2: When create_virtual_network=false, nat_gateway_public_ips is always [] even though the cluster really does egress through the supplied subnet's NAT gateway, so consumers wiring allowlists off this output get an empty set. The output is accurate to what the module itself creates, but the description ('Egress addresses of the cluster') overstates it in the BYO-network path. Note the caveat in the output description, e.g. that it only reflects a NAT gateway the module created.</violation>
</file>

<file name="deployment/terraform/modules/azure/onyx/variables.tf">

<violation number="1" location="deployment/terraform/modules/azure/onyx/variables.tf:190">
P3: The postgres_password description says enable_entra_database_authentication turns password logins off, but that variable only adds Entra logins on top of passwords; entra_database_authentication_only is the one that disables them, and the validation checks that one. Fix the description so it names the correct variable and doesn't mislead users into skipping the password by setting enable_entra_database_authentication alone.</violation>
</file>

<file name="deployment/terraform/modules/azure/onyx/main.tf">

<violation number="1" location="deployment/terraform/modules/azure/onyx/main.tf:20">
P2: When the normalized name prefix reaches 18 characters, this truncation keeps only three of the six digest characters, so distinct deployments can collide in Azure's global storage-account namespace. Reserve 15 characters for the prefix so the full digest remains in the 24-character name.</violation>
</file>

<file name="deployment/terraform/modules/azure/onyx/tests/onyx.tftest.hcl">

<violation number="1" location="deployment/terraform/modules/azure/onyx/tests/onyx.tftest.hcl:273">
P3: This assertion is tautological: it only restates `postgres_password = null` from the run's own variables block, so it can never fail and verifies nothing about the module. The meaningful part of this test is that the plan succeeds without a password; drop the redundant assert, or replace it with one that checks a real signal from the module (e.g. `var.entra_database_authentication_only`).</violation>
</file>

<file name="deployment/terraform/modules/azure/README.md">

<violation number="1" location="deployment/terraform/modules/azure/README.md:273">
P1: Require the Helm release to be installed in the `onyx` namespace, for example with `--namespace onyx --create-namespace`. The Terraform-created `onyx-workload-access` ServiceAccount is namespace-scoped, so installing elsewhere makes the API and Celery pods reference a nonexistent account and fail to start.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

greptile-apps · 2026-08-19T23:38:00Z

+variable "create_workload_service_account" {
+  type        = bool
+  description = "Create the workload service account. Turn this off when the Helm chart already creates it."
+  default     = true


Service-account opt-out drops namespace

When a caller sets create_workload_service_account=false so Helm can create the service account while leaving create_workload_namespace=true, the AKS module also skips namespace creation. The documented Helm command does not pass --create-namespace, so installation fails because the onyx namespace does not exist.

Prompt To Fix With AI

This is a comment left during a code review. Path: deployment/terraform/modules/azure/onyx/variables.tf Line: 396-399 Comment: **Service-account opt-out drops namespace** When a caller sets `create_workload_service_account=false` so Helm can create the service account while leaving `create_workload_namespace=true`, the AKS module also skips namespace creation. The documented Helm command does not pass `--create-namespace`, so installation fails because the `onyx` namespace does not exist. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

github-actions · 2026-08-20T21:10:49Z

Preview Deployment

Status	Preview	Commit	Updated
✅	https://onyx-preview-gmf44lv26-danswer.vercel.app	`e45b86a`	2026-08-20 21:10:48 UTC

Wires the six modules together with t-shirt sizing, and documents the set. Completes the Azure equivalent of deployment/terraform/modules/aws. Sizing picks the Azure size closest to what the AWS composition chooses at each tier, with one deliberate departure: the index pool is memory-optimised at every tier, because on Azure it carries the document index itself. There is no managed OpenSearch to move that load off the cluster. Two things the composition does that no single module can: - Flow logs get a storage account of their own. That keeps the dependency one-way - log account, then network, then the file store account that restricts itself to the network's subnets - where pointing the network at the file store account would have made a cycle. - Egress picks itself. With a created network and a NAT gateway the cluster routes through it and keeps one address; bringing a network without one falls back to letting AKS manage outbound. Unlike the AWS composition this module declares no provider block. The azurerm provider needs a features block and a subscription, and both belong to the root module rather than to a module that might be counted. Flow logs are off by default, where the AWS modules have them on. Azure writes them to a storage account and needs a Network Watcher in the region, so a default of on would fail an apply on any subscription without one. The README covers turning them on.

justin-tahara requested a review from a team as a code owner August 19, 2026 22:19

greptile-apps Bot reviewed Aug 19, 2026

View reviewed changes

Comment thread deployment/terraform/modules/azure/onyx/variables.tf

Comment thread deployment/terraform/modules/azure/README.md

cubic-dev-ai Bot reviewed Aug 19, 2026

View reviewed changes

justin-tahara force-pushed the jtahara/azure-tf-onyx branch from 8c256a0 to 4d82f9a Compare August 19, 2026 22:48

greptile-apps Bot reviewed Aug 19, 2026

View reviewed changes

Comment thread deployment/terraform/modules/azure/README.md Outdated

cubic-dev-ai Bot reviewed Aug 19, 2026

View reviewed changes

justin-tahara force-pushed the jtahara/azure-tf-onyx branch from 4d82f9a to 812cdb4 Compare August 19, 2026 23:14

greptile-apps Bot reviewed Aug 19, 2026

View reviewed changes

Comment thread deployment/terraform/modules/azure/onyx/main.tf

justin-tahara force-pushed the jtahara/azure-tf-onyx branch from 812cdb4 to 6e04fb5 Compare August 19, 2026 23:31

greptile-apps Bot reviewed Aug 19, 2026

View reviewed changes

jmelahman approved these changes Aug 20, 2026

View reviewed changes

justin-tahara force-pushed the jtahara/azure-tf-onyx branch from 6e04fb5 to e45b86a Compare August 20, 2026 21:06

justin-tahara force-pushed the jtahara/azure-tf-onyx branch from e45b86a to 1319be2 Compare August 20, 2026 21:12

justin-tahara force-pushed the jtahara/azure-tf-onyx branch from 1319be2 to ff8a30e Compare August 20, 2026 21:20

justin-tahara added this pull request to the merge queue Aug 20, 2026

Merged via the queue into main with commit b77e759 Aug 20, 2026
47 of 89 checks passed

justin-tahara deleted the jtahara/azure-tf-onyx branch August 20, 2026 21:25

Navigation Menu

Uh oh!

feat(terraform): add the Azure onyx composition and README - #14105

feat(terraform): add the Azure onyx composition and README#14105
justin-tahara merged 1 commit into
jtahara/azure-tf-waffrom
jtahara/azure-tf-onyx

justin-tahara commented Aug 19, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot Aug 19, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

justin-tahara commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

One deliberate parity gap, for a decision

How Has This Been Tested?

Additional Options

Changes from review (greptile, cubic)

Round 2

Round 3

Uh oh!

greptile-apps Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

justin-tahara commented Aug 19, 2026 •

edited

Loading

greptile-apps Bot commented Aug 19, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading